Skip to content

[Fix] Fixed syncing of effective fields in plugin framework implementation of share resource - #4969

Merged
tanmay-db merged 15 commits into
mainfrom
panic-share-pluginfw
Oct 14, 2025
Merged

tanmay-db merged 15 commits into
mainfrom
panic-share-pluginfw

Conversation

@tanmay-db

@tanmay-db tanmay-db commented Aug 22, 2025 •

Copy link
Copy Markdown
Contributor

Changes

Planned objects can be less than the state objects. This can happen when someone removes an object in share resource outside the terraform (for example through UI). The changes fixes the syncing of effective fields by making sure we iterate over the planned and state objects properly.

Ref: #4913

Tests

Integration tests

Unit tests

  • Tests panics before the changes (ref):
image - Tests passes over the changes

@tanmay-db
tanmay-db requested review from a team as code owners August 22, 2025 14:14
@tanmay-db
tanmay-db requested review from renaudhartert-db and removed request for a team August 22, 2025 14:14
@tanmay-db
tanmay-db removed request for a team and renaudhartert-db August 22, 2025 14:15
@tanmay-db tanmay-db changed the title [Fix] Fix syncing effective fields in share resource [Fix] Fixed syncing of effective fields in plugin framework implementation of share resource Aug 22, 2025
for j := range planObjects {
if stateObjects[i].Name == planObjects[j].Name {
mode.objectLevel(ctx, &stateObjects[i], planObjects[j])
finalObjects = append(finalObjects, stateObjects[i])

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what happens if the plan as an object which is not in the state? It seems that we are ignoring it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we ignore it because in terraform we want to create the infra from the config

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit more what is what? I think I am mixing what stateObjects, planObjects and finalObjects are.
As it is, it feels that we are storing (state) a different set of objects that we are creating (plan).

@@ -446,8 +446,13 @@ func (r *ShareResource) syncEffectiveFields(ctx context.Context, plan, state Sha
stateObjects, _ := state.GetObjects(ctx)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we suppressing the error? Can this fail?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rauchy
rauchy force-pushed the panic-share-pluginfw branch from 006c174 to 881c81c Compare August 28, 2025 11:02
@rauchy
rauchy temporarily deployed to test-trigger-is August 28, 2025 11:02 — with GitHub Actions Inactive
@rauchy
rauchy temporarily deployed to test-trigger-is August 28, 2025 11:03 — with GitHub Actions Inactive
@github-actions

Copy link
Copy Markdown
Contributor

If integration tests don't run automatically, an authorized user can run them manually by following the instructions below:

Trigger:
go/deco-tests-run/terraform

Inputs:

  • PR number: 4969
  • Commit SHA: b9c0fc0360ccd420007f307e42163127fb075c7a

Checks will be approved automatically on success.

@mgyucht mgyucht left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

// syncEffectiveFields syncs the effective fields between existingState and newState
// and returns the newState
//
// existingState: infrastructure values that are recorded in the existing terraform state.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This corresponds to the state in Read method and plan in the Create/Update methods.

Comment thread internal/providers/pluginfw/products/sharing/resource_share.go
Comment thread internal/providers/pluginfw/products/sharing/resource_share.go
@tanmay-db
tanmay-db added this pull request to the merge queue Oct 14, 2025
Merged via the queue into main with commit 0c0272a Oct 14, 2025
12 checks passed
@tanmay-db
tanmay-db deleted the panic-share-pluginfw branch October 14, 2025 10:21
deco-sdk-tagging Bot added a commit that referenced this pull request Oct 15, 2025
## Release v1.92.0

### Breaking Changes

* Remove stale resources/datasources/documentation related to Clean Room services.
* databricks\_permissions resource no longer updates permissions on delete. This is to mitigate an issue with incorrect IS\_OWNER being set ([#5096](#5096))

### New Features and Improvements

* Add `arm` option to `databricks_node_type` instead of `graviton` ([#5028](#5028))
* Add `data_quality_monitor` resource and data sources ([#5092](#5092)).
* Add `data_quality_refresh` resource and data sources ([#5092](#5092)).
* Perform workspace-level permission assignment by `user_name`, `group_name`, or `service_principal_name` ([#5068](#5068)).

### Bug Fixes

* Fixed syncing of effective fields in plugin framework implementation of share resource ([#4969](#4969))
* Mark `storage_location` as read-only in `databricks_catalog` ([#5075](#5075))

### Documentation

* Add instructions for migration from deprecated `databricks_catalog_workspace_binding` to `databricks_workspace_binding` ([#5054](#5054))
* Document output attributes in `databricks_storage_credential` ([#5093](#5093))

### Internal Changes

* Bump the Go SDK to v0.86.0 ([#5092](#5092)).
github-merge-queue Bot pushed a commit that referenced this pull request Oct 16, 2025
…efault (#5089)

## Changes
<!-- Summary of your changes that are easy to understand -->
Make the plugin framework implementation of share resource as default.
With this change, `databricks_share` will use the plugin framework
implementation.

To be merged after:
#4969

Note: Removal of SDKv2 implementation will be done in next release so
users can fallback to SDKv2 implementation in case there are some
issues.

## Tests
<!--
How is this tested? Please see the checklist below and also describe any
other relevant tests
-->
Integration tests
- Applying: SDKv2 -> Plugin Framework 
- Applying: Plugin Framework -> SDKv2

This branch was previously deployed

1 inactive deployment
test-trigger-is — b9c0fc03 Deployed Oct 11, 2025 by tanmay-db via Trigger Tests #2458
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants